Fix: missing early fail for basic auth without credentials#60029
Fix: missing early fail for basic auth without credentials#60029icarta-l wants to merge 1 commit into
Conversation
icarta-l
requested review from
CarlSchwan,
icewind1991,
leftybournes and
salmart-dev
and removed request for
a team
| $this->session->close(); | ||
| return true; | ||
| } else { | ||
| if (empty($username) && empty($password)) { |
There was a problem hiding this comment.
| if (empty($username) && empty($password)) { | |
| if ($username === '' && $password === '') { |
Also this should probably be a ||?
There was a problem hiding this comment.
Thank you for the review and suggestion.
I have just forced push the branch to include your suggestions after rebasing it with the master branch.
I do agree with the || instead of &&. I used the && because it was a request from the author of the issue this pull request is trying to resolve but the || makes sense to me.
One question: why do you check for empty strings instead of using the empty() function?
There was a problem hiding this comment.
The empty does a lot of things and can cause unexpected results. Generally it is better to always be as explicit as possible about what you want to check, to avoid any ambiguities.
icarta-l
force-pushed
the
fix/missing-early-fail-for-basic-auth-without-credentials
branch
from
35c25c9 to
96e8877
Compare
| private Manager $twoFactorManager, | ||
| private IThrottler $throttler, | ||
| string $principalPrefix = 'principals/users/', | ||
| string $principalPrefix = 'principals/users/' |
There was a problem hiding this comment.
Please remove the unrelated change.
icarta-l
force-pushed
the
fix/missing-early-fail-for-basic-auth-without-credentials
branch
from
96e8877 to
6790acd
Compare
Make "validateUserPass" method of OCA\DAV\Connector\Sabre\Auth class return false after checking if user is logged in if empty username or password have been passed to it Fixes nextcloud#59849 Signed-off-by: Idan <cartaidan@gmail.com>
icarta-l
force-pushed
the
fix/missing-early-fail-for-basic-auth-without-credentials
branch
from
6790acd to
cedeeed
Compare
2 participants
Fixes #59849
Make "validateUserPass" method of OCA\DAV\Connector\Sabre\Auth class return false if empty username and password have been passed to it
Summary
TODO
Checklist
3. to review, feature component)stable32)AI (if applicable)